Read the cargo config chain once (#1832) by mikolalysenko · Pull Request #1849 · SocketDev/socket-patch

Mikola Lysenko (mikolalysenko) · 2026-10-29T18:57:25Z

LLM Description written by Claude Code:claude-opus-5-5

Summary

The cargo config chain was walked twice. cargo_build_cache::ConfigDirs::read preferred .cargo/config.toml when both config and config.toml existed. Cargo, and source_replacement::config_layer_paths, prefer the extensionless config. So after apply/rollback the build-cache cleaner removed fingerprints from a target dir cargo never used, and cargo kept linking stale code. This is register row A01 (register; living document Part 3, duplication).

I checked cargo's precedence with real cargo 1.93: warning: both .cargo/config and .cargo/config.toml exist. Using .cargo/config.

Changes

One walk. ConfigDirs::read now takes its file list from config_layer_paths. I deleted its own ancestor walk, its $CARGO_HOME push and its file pick, keeping only the [build] target-dir/build-dir lookup. Two side effects: $CARGO_HOME is no longer read twice when an ancestor's .cargo is that directory, and the $CARGO_HOME base falls back to the home itself, not "", when it has no parent.
Relative cwd. /code-review high found this in the touched function; it was already broken on main. --cwd defaults to ., and Path::new(".").ancestors() stops at ., so from a workspace member the cleaner found neither the workspace root nor a parent .cargo/config, and invalidated no build dir. build_dirs_with now walks std::path::absolute(cwd) with .. folded by relpath::normalize_lexically_keeping_escapes. This is the same fold cargo_crawler::resolve_for does. Without the fold, --cwd ../app would walk the directory it left, which Bugbot caught.
Probe agrees with the read. The file probe asks group_commit::exists first, as read_regular_to_string_sync does. The old code chose the file by attempting the capture-aware read.
Doc comment. The ConfigDirs comment no longer restates the (wrong) file order; it points at config_layer_paths.

Tests

build_dirs_read_the_legacy_config_when_both_exist: with both files present, the build dirs are exactly [legacy]. Red on the old pick: left: [".../ws/modern"] right: [".../ws/legacy"].
build_dirs_walk_above_a_relative_cwd (#[serial], chdir, as nuget_config::a_relative_start_reads_parent_configs does). "." from a member finds the parent config's out; red without absolute: left: [] right: [".../ws/out"]. "../ws/member" from a sibling ignores the sibling's build-dir; red without the fold: left: [ws/out, left/wrong].
build_dirs_honours_env_config_and_workspace_root and the source_replacement tests are unchanged and pass.

Size

git diff --stat: 1 file, +78/−16. Production is about +12/−16 and tests +66.

Behavior change

Bug fixes only: the cleaner now targets the directory cargo builds into when both config files exist and when --cwd is relative. No output bytes, JSON shapes or exit codes change.

Review findings not fixed

Unreadable chosen file drops the layer instead of falling back to the sibling. Cargo itself errors on an unreadable config, so a sibling fallback matches no real build. Not worth the code.
No $CARGO_HOME-dedup test here. config_layer_paths has its own tests in source_replacement.rs.
Probe plus read is two syscalls per file. A few stats per apply, and the same probe the crawler uses.

Verification

cargo clippy --workspace --all-features -- -D warnings is clean.
cargo test -p socket-patch-core --lib: 6991 passed and 4 failed. All 4 are read-only-permission tests in copy_tree, vlt_heal, pypi_poetry and pypi_requirements that fail only because the sandbox runs as root. They are unrelated to this diff.
CI was green on the first push (82 checks). On 2c715eb clippy passed, but coverage, e2e-build, lint-ecosystems and dispatch-tests lost their Depot runners twice (no test failed); a later re-run is fully green. This adds no cfg(windows)/cfg(unix) code.

Risk

Low. It touches one private function in one file, whose callers are apply and rollback via invalidate_project.

🤖 Generated with Claude Code

https://claude.ai/code/session_011f9aeQ57i1EpKYcB1Ks8rf

Note

Low Risk
Scoped to private build-directory discovery used by apply/rollback cache invalidation; behavior fixes only, with no auth or data-path changes.

Overview
Fixes build-cache invalidation after apply/rollback so it clears fingerprints in the same target / build-dir cargo actually uses.

ConfigDirs::read no longer walks .cargo config on its own. It reuses config_layer_paths, matching cargo’s precedence (.cargo/config over .cargo/config.toml when both exist). That stops the cleaner from wiping the wrong directory and leaving stale .rlib links.

build_dirs_with now resolves a relative --cwd (e.g. . or ../ws/member) via absolute + lexical normalize before ancestor walks, so workspace roots and parent configs are found when run from a member crate. Config file probing uses the same group_commit::exists check as reads elsewhere.

Two tests lock in legacy-config precedence and relative-cwd discovery; no CLI output or exit-code changes.

^{Reviewed by Cursor Bugbot for commit 2c715eb. Configure here.}

Generated by Claude Code

Assisted-by: Claude Code:claude-opus-5-5

The build-cache cleaner walked the .cargo/config chain itself and preferred config.toml when both config and config.toml existed. Cargo reads the extensionless file, so the cleaner removed fingerprints from a target dir cargo never used. ConfigDirs::read now takes its file list from source_replacement::config_layer_paths, which the source replacement reader already uses, and keeps only the [build] lookup. It also stops reading $CARGO_HOME twice when an ancestor's .cargo is it. Fixes #1832 Assisted-by: Claude Code:claude-opus-5-5

build_dirs walked the ancestors of the cwd as given, and --cwd defaults to ".", whose ancestors stop at ".". Run from a workspace member, the cleaner found neither the workspace root nor a parent .cargo/config, so it invalidated no build dir at all. It now walks the absolute path. The config file probe also consults the open group commit, as the read that follows it does, so the two agree on captured files. Assisted-by: Claude Code:claude-opus-5-5

Mikola Lysenko (mikolalysenko) · 2026-10-29T19:34:08Z

BugBot review

Generated by Claude Code

Assisted-by: Claude Code:claude-opus-5-5

cursor

Stale Bugbot comment from a previous run.

std::path::absolute keeps .., so --cwd ../app walked the ancestors of the directory being left and could take its build-dir. Fold the absolute cwd lexically, as cargo_crawler::resolve_for does. Assisted-by: Claude Code:claude-opus-5-5

Mikola Lysenko (mikolalysenko) · 2026-10-29T19:59:20Z

BugBot review

Generated by Claude Code

cursor

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit 2c715eb. Configure here.}

Mikola Lysenko (mikolalysenko) · 2026-10-29T20:16:42Z

[agent] CI on 2c715eb failed in coverage (1), coverage (2), clippy and e2e-build (ubuntu-latest), and cancelled lint-ecosystems and dispatch-tests. None of these failures comes from this PR.

Each failed job's annotation reads "The self-hosted runner lost communication with the server", or, for lint-ecosystems, "exceeded the maximum execution time of 10m0s".
No failed step ran a test body.
The same jobs passed on the previous head, 3b882ea, which also passed all 82 checks. 2c715eb adds only a lexical .. fold and a test.
Locally, clippy with -D warnings and the cargo_build_cache tests pass.

I've re-run the failed jobs once.

Generated by Claude Code

Mikola Lysenko (mikolalysenko) · 2026-10-29T20:36:14Z

[agent] The re-run lost its Depot runners again. coverage (1) was cancelled at "Set up job", and coverage (2), e2e-build (ubuntu-latest), lint-ecosystems and dispatch-tests all report "self-hosted runner lost communication". clippy passed. No test failed in any run of 2c715eb. I've used up my one re-run, so a maintainer or the burn-down routine needs to re-run the failed jobs once the runners are healthy.

Generated by Claude Code

Assisted-by: Claude Code:claude-opus-5-5

Mikola Lysenko (mikolalysenko) · 2026-10-29T22:22:22Z

Burn-down agent: labeled Ready for review at 2c715eb185b6868047164544be5c81988aec1934.

CI: 31/31 run checks green (19 skipped). The two earlier attempts lost their Depot runners before any step ran (no test failed); attempt 3 passed cleanly.
Bugbot: reviewed 2c715eb, no findings; no open threads.
Reviewer note: single-file refactor of crates/socket-patch-core/src/utils/cargo_build_cache.rs so the cargo config chain is read once (Cargo build-cache invalidation reads .cargo/config.toml where cargo reads .cargo/config, so it cleans the wrong target dir #1832).

Generated by Claude Code

Mikola Lysenko (mikolalysenko) · 2026-10-29T22:52:20Z

Final review brief

What it does: Fixes #1832. ConfigDirs::read in cargo_build_cache had its own copy of the cargo-config walk and file pick, and that copy preferred config.toml over config. It now reuses source_replacement::config_layer_paths, which prefers the extensionless config the way cargo does. A relative --cwd is now made absolute and .. is folded out before the ancestor walk.

Risk: low. It is one private function, reached only from invalidate_project (apply and rollback). The worst case of a wrong pick is an extra recompile. The $CARGO_HOME fallback is also slightly more robust: the home is read once, and a home with no parent is handled.

Look here:

cargo_build_cache.rs:64-66:`` absolute path plus .. fold
cargo_build_cache.rs:158-163:``` the shared walk
source_replacement.rs:113-141:```` the single file pick
cargo_build_cache.rs:429 and :456:````` the two new tests

Verified: I read the full diff against #1832 and checked the precedence against cargo. The fold keeps root and drive prefixes, and on Windows std::path::absolute already resolves ... CHANGELOG.md is untouched. CI is green on 2c715eb (31 passed, 19 skipped, ci-ok and clippy both green). Bugbot found no new issues.

Changes I made: none.

Open questions: none. One nit: the chdir test restores the working directory without a drop guard, though it does restore it before asserting.

Auto-merge (squash) is armed, so approving sends this straight to the merge queue.

Generated by Claude Code

Wenxin Jiang (Wenxin-Jiang)

[agent] Reviewed 2c715eb185b6868047164544be5c81988aec1934. Checked shared Cargo config precedence, path bases, and relative-cwd ancestor traversal. No major correctness issues found. Validation: source and regression-test review plus the available exact-head CI checks; no local test pass is claimed.

Claude (claude) added 3 commits October 29, 2026 18:57

Start #1832: one cargo config chain

36768d5

Assisted-by: Claude Code:claude-opus-5-5

Mikola Lysenko (mikolalysenko) marked this pull request as ready for review October 29, 2026 19:34

Mikola Lysenko (mikolalysenko) added refactor Structural change: duplicated code or logic, missing abstraction, layering, dead code arch-refactor PR opened by the scheduled architecture refactor routine labels Oct 29, 2026

Mikola Lysenko (mikolalysenko) pushed a commit that referenced this pull request Oct 29, 2026

refactor: run 20261029T193441Z, PR #1849 for A01

2f8a6fe

Assisted-by: Claude Code:claude-opus-5-5

cursor Bot reviewed Oct 29, 2026

View reviewed changes

Comment thread crates/socket-patch-core/src/utils/cargo_build_cache.rs Resolved

Fold .. out of the cwd before the cargo walks

2c715eb

std::path::absolute keeps .., so --cwd ../app walked the ancestors of the directory being left and could take its build-dir. Fold the absolute cwd lexically, as cargo_crawler::resolve_for does. Assisted-by: Claude Code:claude-opus-5-5

cursor Bot reviewed Oct 29, 2026

View reviewed changes

Mikola Lysenko (mikolalysenko) mentioned this pull request Oct 29, 2026

CI performance dashboard #1182

Open

Mikola Lysenko (mikolalysenko) pushed a commit that referenced this pull request Oct 29, 2026

refactor: #1849 CI state

3212845

Assisted-by: Claude Code:claude-opus-5-5

This was referenced Oct 29, 2026

Swarm health manager log #1382

Open

Depot runners losing connection mid-job evict merge-queue entries #1857

Closed

Mikola Lysenko (mikolalysenko) pushed a commit that referenced this pull request Oct 29, 2026

refactor: #1849 green

5345c59

Assisted-by: Claude Code:claude-opus-5-5

Mikola Lysenko (mikolalysenko) added the Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review label Oct 29, 2026

Mikola Lysenko (mikolalysenko) enabled auto-merge October 29, 2026 22:52

Wenxin Jiang (Wenxin-Jiang) approved these changes Nov 2, 2026

View reviewed changes

Mikola Lysenko (mikolalysenko) added this pull request to the merge queue Nov 2, 2026

Wenxin Jiang (Wenxin-Jiang) approved these changes Nov 2, 2026

View reviewed changes

Merged via the queue into main with commit dcf575e Nov 2, 2026
103 of 116 checks passed

Mikola Lysenko (mikolalysenko) deleted the arch-refactor/1832-cargo-config-chain branch November 2, 2026 16:02

Navigation Menu

Repository navigation

Read the cargo config chain once (#1832) - #1849

Read the cargo config chain once (#1832)#1849
Mikola Lysenko (mikolalysenko) merged 4 commits into
mainfrom
arch-refactor/1832-cargo-config-chain

Mikola Lysenko (mikolalysenko) commented Oct 29, 2026 •

edited

Loading

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 29, 2026

Uh oh!

cursor Bot left a comment •

edited

Loading

Uh oh!

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 29, 2026

Uh oh!

cursor Bot left a comment

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 29, 2026

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 29, 2026

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 29, 2026

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 29, 2026

Uh oh!

Wenxin Jiang (Wenxin-Jiang) left a comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

Mikola Lysenko (mikolalysenko) commented Oct 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

LLM Description written by Claude Code:claude-opus-5-5

Summary

Changes

Tests

Size

Behavior change

Review findings not fixed

Verification

Risk

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 29, 2026

Uh oh!

cursor Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 29, 2026

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 29, 2026

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 29, 2026

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 29, 2026

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 29, 2026

Final review brief

Uh oh!

Wenxin Jiang (Wenxin-Jiang) left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Mikola Lysenko (mikolalysenko) commented Oct 29, 2026 •

edited

Loading

cursor Bot left a comment •

edited

Loading